Skip to content

fix: use correct POSIX signal exit convention for KeyboardInterrupt in test.py#5

Closed
Dhakshin2007 wants to merge 1 commit intofacebook:mainfrom
Dhakshin2007:fix/keyboard-interrupt-exit-code
Closed

fix: use correct POSIX signal exit convention for KeyboardInterrupt in test.py#5
Dhakshin2007 wants to merge 1 commit intofacebook:mainfrom
Dhakshin2007:fix/keyboard-interrupt-exit-code

Conversation

@Dhakshin2007
Copy link
Copy Markdown
Contributor

Bug

In test.py, the KeyboardInterrupt handler does:

except KeyboardInterrupt:
    sys.exit(signal.SIGINT)

signal.SIGINT has the integer value 2. So this is equivalent to sys.exit(2) — a plain non-zero error exit. This is incorrect because:

  • Shells (bash, zsh, etc.) use exit code 128 + N to indicate a process was killed by signal N. For SIGINT, the expected exit code is 130 (128 + 2).
  • CI runners (GitHub Actions, etc.) inspect exit codes to determine if a job was cancelled/interrupted vs. failed with an error. An exit code of 2 is treated as a generic error, not an interrupt.
  • Pipeline chaining — if this script is part of a shell pipeline, the wrong exit code means $? and set -e behave incorrectly.

Fix

Reset the SIGINT handler to the OS default and re-raise the signal. The OS then terminates the process with the correct exit status (130) automatically:

except KeyboardInterrupt:
    signal.signal(signal.SIGINT, signal.SIG_DFL)
    os.kill(os.getpid(), signal.SIGINT)

This is the idiomatic Python pattern for correct SIGINT propagation (referenced in Python docs and PEP recommendations).

Impact

This is a correctness bug that affects CI behaviour — on Ctrl+C or SIGINT from a test runner, the script exits with code 2 (error) instead of 130 (interrupted). This can cause CI pipelines to report a test failure instead of a cancellation.

Using sys.exit(signal.SIGINT) exits with code 2, which shells and CI
systems interpret as a regular error, not a signal-terminated process.

The POSIX convention for a process terminated by signal N is to exit
with code 128 + N. For SIGINT (N=2), this is exit code 130. The
correct idiomatic approach is to reset SIGINT to its default handler
and re-raise it — the OS then sets the exit status automatically,
allowing shells and CI runners to correctly detect that the process
was interrupted rather than failing with a generic error code.
@meta-cla meta-cla Bot added the cla signed label Apr 20, 2026
@meta-codesync
Copy link
Copy Markdown

meta-codesync Bot commented Apr 20, 2026

@brittanyrey has imported this pull request. If you are a Meta employee, you can view this in D101664858.

@brittanyrey brittanyrey self-assigned this Apr 20, 2026
@meta-codesync meta-codesync Bot closed this in 3572844 Apr 20, 2026
@meta-codesync
Copy link
Copy Markdown

meta-codesync Bot commented Apr 20, 2026

@brittanyrey merged this pull request in 3572844.

@brittanyrey
Copy link
Copy Markdown
Contributor

Thank you for your contribution to Lifeguard!

@Dhakshin2007
Copy link
Copy Markdown
Contributor Author

Thank you for your contribution to Lifeguard!

Always, Thanks for Prompt Response ♥️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants